-
Notifications
You must be signed in to change notification settings - Fork 35
Update to support EventBus7 #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 5.x
Are you sure you want to change the base?
Conversation
Do old Forge versions (1.21.1-1.21.5) use EB7? This could be a breaking change that I don't want to merge into KFF 6.x |
But, it also breaks support for MC 1.21.6-1.21.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll review the KotlinModContainer class later, I'm suspicious of the module reflection, but knowing LexForge, it's probably just how they decided to do it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all of these random Gradle options? If these changes are just for your setup, please remove them from the PR
|
||
min_mc_version = 1.20.6 | ||
unsupported_mc_version = 1.22 | ||
|
||
# KOTLIN FOR FORGE VERSION | ||
kff_version=5.9.0 | ||
kff_max_version=6.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 7.0.0, not 6.1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed that
@@ -132,9 +133,9 @@ public object AutoKotlinEventBusSubscriber { | |||
|
|||
private fun registerTo(any: Any, target: Mod.EventBusSubscriber.Bus, mod: KotlinModContainer) { | |||
if (target == Mod.EventBusSubscriber.Bus.FORGE) { | |||
target.bus().get().register(any) | |||
target.bus().get()?.register(MethodHandles.lookup(), any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the bus supplier now nullable? Should this log an error if the bus is null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I use !! or ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler 2.2.0 won't let me go without !! or ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this change from the PR
Is it ok now? |
ok
…On Sun, Jul 13, 2025 at 6:39 AM thedarkcolour ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I'll review the KotlinModContainer class later, I'm suspicious of the
module reflection, but knowing LexForge, it's probably just how they
decided to do it...
------------------------------
On gradle.properties
<#129 (comment)>
:
Why all of these random Gradle options? If these changes are just for your
setup, please remove them from the PR
------------------------------
In gradle.properties
<#129 (comment)>
:
>
min_mc_version = 1.20.6
unsupported_mc_version = 1.22
# KOTLIN FOR FORGE VERSION
-kff_version=5.9.0
-kff_max_version=6.0.0
This should be 7.0.0, not 6.1.0
------------------------------
In
src/kfflang/forge/kotlin/thedarkcolour/kotlinforforge/AutoKotlinEventBusSubscriber.kt
<#129 (comment)>
:
> @@ -132,9 +133,9 @@ public object AutoKotlinEventBusSubscriber {
private fun registerTo(any: Any, target: Mod.EventBusSubscriber.Bus, mod: KotlinModContainer) {
if (target == Mod.EventBusSubscriber.Bus.FORGE) {
- target.bus().get().register(any)
+ target.bus().get()?.register(MethodHandles.lookup(), any)
Why is the bus supplier now nullable? Should this log an error if the bus
is null?
------------------------------
On
src/kfflang/forge/resources/META-INF/services/net.minecraftforge.forgespi.language.IModLanguageProvider
<#129 (comment)>
:
Please remove this change from the PR
—
Reply to this email directly, view it on GitHub
<#129 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BK6AZOOB5WOF5UYTEULFC733IHWK5AVCNFSM6AAAAACBL6TRCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMJTHA4DCOJSGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I'm not yet done with the update. Still testing and making changes |
Currently fighting with EventBusSubscriber firing too late for our likings |
I can also take a look at it after I'm back from work. |
As of now, all Events to be automatically registered need to be @SubscriveEvent and @JvmStatic |
…scribers. ADDED ACTUAL EVENT FIRING AND MANY MORE
Ok. Done with the PR. DOne with the commits, done with the updating. All that's left is for you to merge. Everything WOrks perfectly fine |
Or can I upload to curseforge?? and modrinth?? PLS |
Right Now, All EventBusSubscriber classes have to use KotlinEventBusSubscriber, all Mod KotlinMod all Bus KotlinBus and the @subscribe methods should NOT have @JvmStatic |
until you publish the changes to curseforge I'll embed them |
Gonna look this over in a few hours. |
Updated KotlinForForge to support MC 1.21.7 and the new EventBus7 introduced in Forge 57.x.
Mod containers now use BusGroup for event dispatching, keeping up with the Forge changes.